-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved Visual Cohesion on About Us Page #113
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for opening a pull request @admwln. We'll will review it as soon as possible 🙌❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @admwln. Could you please provide a screenshot of the result.
Hey @Virtual4087 Sure thing! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but I think changing the bg color itself would make it better. Could you try that once? If not, I'll merge this pr cause this doesn't look bad either. Try making the "about us" block not stand out too much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a ss of your changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but there seems to be some conflict in your branch cause I just merged another pr
Oh I see, it seems that pr #121, which is already merged to main, fixes issue #107 (that the present pr tries to solve). The two prs are not compatible. It's one or the other, as far as I can tell. I have no problem with you choosing the other solution over mine. It was a fun exercise! Looking at the newly merged changes, it seems that pr #121 fixes the problem that's depicted in my last screenshot, so it might be preferable regardless. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@admwln True but I don't want to not merge your changes after you've already done so much. So maybe you could create another fork and just change the text format like color and stuff to make it more clean? The current "About Us" section's text content is not very appealing. If you can you can make it more minimalistic.
Thank you, @Virtual4087. See pr #122. |
@admwln Done I'll be closing this pr. Thanks for the help. |
Fixes #107 : Change About Us page design
I have enhanced the visual cohesion between the background of the About Me page and the foreground content by:
This change helps unify the background and foreground elements, creating a smoother and more cohesive design. The effect is subtle but noticeable, and it improves the overall aesthetics of the page without being distracting.
Testing: